-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plugin: track the resources used across all of an association's running jobs #561
plugin: track the resources used across all of an association's running jobs #561
Conversation
e42e60e
to
a4fbc4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 two little questions.
src/plugins/mf_priority.cpp
Outdated
b->cur_run_jobs--; | ||
if (jobspec) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
under what conditions would jobspec
be NULL? Just wondering because then a user might get stuck with the system permanently thinking they have active resources when in fact they don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only case I can think of where jobspec
might be NULL is if for some reason the jobtap plugin failed to unpack the jobspec
in flux_plugin_arg_unpack ()
. Perhaps the plugin should raise an exception on the job if it fails to unpack it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure that can't happen - the "pack" operation would fail and not call this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you said unpack (sorry). You probably want to immediately fail if the unpack fails (like due to out of memory).
For running jobs, would it make more sense to grab R instead of jobspec? In jobspec, the node count can be ambiguous if the user only requested cores, for example. But R always contains the actual count. |
Problem: flux-core has some useful code for counting the resources for a job, but it needs to be manually copied over. Copy the jj.* files from flux-core and place them in the same folder as the priority plugin so that it can be compiled together and have its methods able to be accessed by the plugin. Manually copy over the streq() definition and place it in jj.hpp as well as copy over its LICENSE from flux-core. Reference it in jj.hpp.
Problem: An Association object has no way to track its current node and core count across all of its running jobs, which is needed in order to enforce limits in case a submitted job would put them over their max. Add two new members to the Association class, cur_nodes and cur_cores, which will store the current number of nodes and cores allocated across all of the association's running jobs, respectively. Update the unit test to account for the addition of the two new members added to the Association class.
Problem: The priority plugin does not keep track of the resources allocated per-job when an association submits a job, which will be needed when the plugin begins to enforce limits on how many resources an association can use across all of their running jobs. Add an increment/decrement of node+core counts in job.state.run and job.state.inactive by unpacking the jobspec of the job and calculating the number of nodes and cores assigned with the job. Increment and decrement these values in the "cur_nodes" and "cur_cores" attributes of the Association object.
Problem: There exists no tests for tracking the number of resources used across an association's running jobs and ensuring that the counts are as expected. Add some tests.
@garlick that's a good suggestion! I picked jobspec because I was able to use the Also, when the plugin ultimately needs to enforce a limit due to exceeding a certain max, I believe this check would need to happen in |
a4fbc4e
to
4494d3c
Compare
Yes, R is only available at RUN state. On node scheduled, homogeneous systems, jobspec should be fine. That's everything we have in production today AFAIK. However for core scheduled and/or homogeneous systems (different node:core counts depending on the node), if the user requests N cores, you may not know how many nodes that is until the scheduler assigns them. Maybe that's OK for now where we are now! |
Thank you for the clarification! Yes, the latter case is absolutely something to consider - I think at this point, just to make some progress on this front, maybe using jobspec (and/or making an estimation as best we can) should be okay. |
Problem
The priority plugin does not keep track of the resources used by an association across all of their running jobs, which will be needed when the plugin begins to enforce limits on how many resources they can have across all of their running jobs.
This PR looks to add the tracking component of an association's resource consumption across all of their running jobs. It does this by copying over the jj (along with a required streq function) code over from flux-core and utilizing the
jj_get_counts_json ()
function to get a totalnnodes
andncores
count. When a job entersjob.state.run
, the association's currentnnodes
andncores
count will get incremented accordingly, and subsequently decremented when the job entersjob.state.inactive
.I've also added some basic tests that check tracking resources across an association's set of jobs as they go through their lifecycle.
I probably need to hold this PR as [WIP] until I properly add the license for the code I copied over fromccan
over in flux-core; for now, I just manually copied thestreq ()
function to thejj.h
file just to get a working solution, but should clean this up.